fix(opencode): use relative SDD prompt paths - #1252
Conversation
|
Maintainers, please add the |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughSDD profile overlays and OpenCode prompt injection now generate settings-relative ChangesPortable prompt reference generation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ff49243 to
2242051
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/components/sdd/profiles_test.go (1)
778-797: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert portability independently of the production helper.
The test computes
expectedRefwith the sameSharedPromptFileRefused byGenerateProfileOverlay, so a regression that reintroduces absolute home paths can make both sides agree. Also assert that generated references are relative and contain nohomepath.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/components/sdd/profiles_test.go` around lines 778 - 797, Update the test around GenerateProfileOverlay and its phase loop to validate portability independently of SharedPromptFileRef: assert each generated prompt reference is relative and does not contain the home path before or alongside comparing expectedRef. Keep the existing expected-reference comparison, but ensure the assertions would fail if GenerateProfileOverlay emits an absolute home-based path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@internal/components/sdd/profiles_test.go`:
- Around line 778-797: Update the test around GenerateProfileOverlay and its
phase loop to validate portability independently of SharedPromptFileRef: assert
each generated prompt reference is relative and does not contain the home path
before or alongside comparing expectedRef. Keep the existing expected-reference
comparison, but ensure the assertions would fail if GenerateProfileOverlay emits
an absolute home-based path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 917aaee2-f391-4db7-843a-71f771119398
⛔ Files ignored due to path filters (1)
testdata/golden/sdd-opencode-multi-settings.goldenis excluded by!testdata/**
📒 Files selected for processing (9)
internal/cli/sync_test.gointernal/components/golden_test.gointernal/components/sdd/inject.gointernal/components/sdd/inject_test.gointernal/components/sdd/profiles.gointernal/components/sdd/profiles_lifecycle_test.gointernal/components/sdd/profiles_test.gointernal/components/sdd/prompts.gointernal/components/sdd/prompts_test.go
dnlrsls
left a comment
dnlrsls
left a comment
There was a problem hiding this comment.
SharedPromptFileRef is still not portable for OpenCode's supported XDG layout. SettingsPath can be $XDG_CONFIG_HOME/opencode/opencode.json, while SharedPromptDir remains $HOME/.config/opencode/prompts/sdd. On Unix this can generate a relative path containing the original home/user topology, and on Windows different config and home volumes make filepath.Rel fail and abort injection.
Please derive settings and shared prompt locations from one resolved config-root model, or otherwise handle roots that cannot produce a portable relative reference. A focused test for absolute XDG_CONFIG_HOME plus Windows different-volume behavior would establish the contract. The standard OpenCode and Kilocode layouts otherwise look correct.
2242051 to
6ddf8d1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/components/sdd/inject_test.go`:
- Around line 2388-2391: Update the expected prompt value in the test assertion
to use the existing SharedPromptFileRef symbol instead of a hardcoded path
string, preserving the comparison and failure message so path separators remain
platform-independent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9b13106d-814f-4a34-bdfa-d97f97eee299
⛔ Files ignored due to path filters (1)
testdata/golden/sdd-opencode-multi-settings.goldenis excluded by!testdata/**
📒 Files selected for processing (10)
internal/cli/sync_test.gointernal/components/golden_test.gointernal/components/sdd/inject.gointernal/components/sdd/inject_test.gointernal/components/sdd/issue557_test.gointernal/components/sdd/profiles.gointernal/components/sdd/profiles_lifecycle_test.gointernal/components/sdd/profiles_test.gointernal/components/sdd/prompts.gointernal/components/sdd/prompts_test.go
|
Updated in The full Go suite, vet, Windows runtime, and E2E checks pass. This is ready for re-review. The only remaining required check is the maintainer-applied |
In OpenCode settings (and derivative adapters such as Kilo Code), prompt paths for SDD phases previously used absolute paths (/home/devel/.gentle-ai/...). This caused issues in containerized, remote, or multi-user environments where /home/devel differs. Updated prompt reference injection to use relative prompt file paths relative to the agent settings file, and updated TestInjectKilocodeSubagentPromptsUseRelativePath to verify using SharedPromptFileRef for cross-platform path separator safety. Fixes Gentleman-Programming#723
Alan-TheGentleman
left a comment
Alan-TheGentleman
left a comment
There was a problem hiding this comment.
Independent final review passed on the exact current candidate.
Direct current-main merge-tree and binary-patch proofs agree, the path fallback remains safe across configured roots and Windows-style paths, generated assets are consistent, focused/package tests pass, and every required non-AI check is green.
fc5dfe2
into
Gentleman-Programming:main
3 participants
Linked Issue
Closes #723
PR Type
type:bug- Bug fix (non-breaking change that fixes an issue)type:feature- New feature (non-breaking change that adds functionality)type:docs- Documentation onlytype:refactor- Code refactoring (no functional changes)type:chore- Build, CI, or tooling changestype:breaking-change- Breaking change (fix or feature that changes existing behavior)Summary
Changes
internal/components/sddinternal/components/golden_test.gotestdata/golden/sdd-opencode-multi-settings.goldenTest Plan
Passed:
go test ./internal/components/sdd/...go test ./internal/components -run '^TestGoldenSDD_OpenCode_Multi$'go test ./internal/componentsgit diff --checkRuntime validation: N/A. The affected output is deterministic generated JSON and is covered by focused unit and golden tests; there is no separate runtime boundary.
Automated Checks
The repository checks will run on this PR.
Contributor Checklist
status:approvedtype:*label is appliedCo-Authored-BytrailersNotes for Reviewers
Size: 9 files, +128/-50 (178 changed lines), under the 400-line review budget.
Summary by CodeRabbit
Bug Fixes
opencode.jsonoverlays and inline prompts use stable, settings-relative{file:...}paths instead of machine-specific absolute paths.Tests